Thanks for the pull request @Tamrat-B!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

Thanks for doing this, this will be incredibly valuable for us!

We can preview video intro of this version at https://youtu.be/0C2fvQXqODQ?t=1793

Hi, I have one small issue.
In I3SDataSource, it hooked Cesium3DTile.prototype.requestContent and rewrote this function by Cesium3DTile.prototype._hookedRequestContent.
It is a smart way, I always want to know how you can find this way. In my situation, if I want to use the same way to load other formats and convert to 3d tiles, there will be a conflict about Cesium3DTile.prototype.requestContent. Only one requestContent function can be working and it depends on the loading sequence.
My suggestion is that, here, we need a register or observer class to make this function virtual.
Thank you for your suggestion. Great Job! @lilleyse

@Tamrat-B thanks to you and team for this pull request!

As we discussed offline, the Cesium team has been heads down on 3D Tiles Next (now ready for community feedback!). As part of this effort, much of the glTF and 3D Tiles engine in CesiumJS was rewritten. That work is now merged into main and available as an experimental API in 1.87.1. This work may impact how we would suggest to architect i3s support.

I think we are at the point where we can spare a few cycles to do a first pass review on this pull request, but if it needs a lot of iteration on feedback, we may be slow as our main focus is on solidifying the draft extensions for 3D Tiles Next.

@lilleyse could you or someone else in the 3D world take an initial look at this?

Perhaps a few things to consider:

• For symmetry, I believe this should be a Primitive, not a DataSource
• Is this reusing intrinsics within CesiumJS reasonably well when possible, e.g., any parts of 3D Tiles LOD selection, cache management, linked lists, the right rendering abstraction layer / transcoding, etc.
  • Are there new intrinsics this could contribute?
• Is this a good foundation for additional i3s support?
• Is the public API and implementation consistent with CesiumJS's coding conventions, unit tests, reference doc, sufficient Sandcastle examples, etc.?
• Are there any new third-party libraries? Are they big? Is the license compatible and documented?
• What is the maintenance implication of this moving forward and who will do it?
  • Is CesiumJS core the right home or is a separate plugin? If i3s support is well done and likely not to be a maintenance issue, CesiumJS core is preferred for the current CesiumJS architecture.

🙏 🙏 🙏

Thanks again for your contribution @Tamrat-B. I've done a pass through this PR and the Sandcastles look great. I have a few comments: mostly looking at the architecture and I've left some suggestions for how some of the components may be better integrated with CesiumJS' existing functionality for things like math, projections, web workers, etc.

@sanjeetsuhag thanks for the review!! we'll incorporate these review changes and let you know once that's done.

Thanks again for your contribution @Tamrat-B!

No one has commented on this pull request in 90 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 90 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@sanjeetsuhag @pjcozzi just to give you an update on this (thank you @cesium-concierge for the gentle reminder :)), we are working on addressing the changes this review is asking for. Some of the asks such as changing it to primitive type, switching to use TaskProcessor, extending dracoloader, maximizing cesium3DTilesetCache are fairly involved and would require significant changes. We are allocating resources to make these changes and will share once that's done (fairly complete) for further review and eventually landing the pr.

@sanjeetsuhag @lilleyse @pjcozzi

Please find changes based on the code review. We have addressed all the requested changes including the architectural changes.

Here is the summary

• I3S geometry payload is now transcoded to glTF (glb) and not b3dm.

• Re-implemented what was previously a DataSource type (I3SDataSource) now as a Primitive (I3SDataProvider):
  I3SDataSource already stored Cesium3dTileset objects internally so for the class to behave as a primitive, we simply needed to add the required functions (update, prePassesUpdate, postPassesUpdate, etc.) and call these functions on the internal tilesets. With this change, an I3S dataset can be directly inserted into a scene's list of primitives and should behave transparently. The class has also been renamed to I3SDataProvider to reflect this change.

• Provided a service based architecture avoiding bloated look up files to convert gravity related (orthometric) height systems to ellipsoidal:
  To seamlessly fuse datasets created on disparate vertical coordinate systems (VCS), we perform geoid conversion of geometric primitives and bounding boxes leveraging existing implementation based on ArcGISTiledElevationTerrainProvider (compiled from Earth Gravitational Model 2008 (EGM2008) Data).The Sandbox examples shows how to set the terrain provider service if required.

• Worker code refactored to make use of TaskProcessor:
  Code to decode i3s geometry has been moved into a separate file (DecodeI3S.js) and will now be called by going through Cesium's TaskProcessor. This allowed us to remove a lot of redundant code by reusing existing functionality from Cesium which was inaccessible with the old worker implementation. DecodeI3S class is now responsible for generating the glTF buffer.

• Removed i3sContentCache:
  This was a redundant caching system because tiles were already being cached by the internal Cesium3dTileset. As long as the memory budget is not exceeded, the transcoded tiles remain loaded, and no conversion needs to be repeated.

• Support debug parameters:
  we added the ability to pass a list of Cesium3dTileset parameters to I3SDataProvider which will pass it along to the internal Cesium3dTileset. This allows the I3sDataProvider to benefit from all existing debug options for Cesium3dTileset like maximumScreenSpaceerror, debugShowBoundingVolume, etc.

• Further refactoring of implementation
  Refactored the implementation to have distinct I3SFeature and I3SNode classes for better accessibility of public classes. I3SNode in cesium translates to a Cesium3DTile whereas I3SFeature class handles I3S Feature data.

• Sandbox samples updated:
  To reflect API usage, 3 sandbox samples have been added. Each sample shows usage of consumption of the supported I3S layer types (3D Object (both textured and untextured) and IntegratedMesh) as well as feature attribute (metadata) selection and display.

• Brief description of the architecture:
  When a scene layer is initialized, and I3SDataProvider.loadURL function is invoked, it processes the 3D Scene Layer of an I3S dataset to create a Cesium 3D Tile Set and loads the root node. When the root node is imported, it creates a Cesium 3D tile that is parented to the Cesium 3D tile set and loads all children of the root node.

  • for each children:
    • Create a place holder 3D tile so that the LOD display can know about it and display as needed
    • When the LOD display decides that a Cesium 3D tile is visible, it invokes requestContent on it.
    • At that moment, we intercept the call to requestContent, and we load the geometry in I3S format.
    • That geometry is then transcoded on the fly to glTF format using TaskProcessor and is ingested by Cesium.
    • When the geometry is loaded, we then load all children of this node as placeholders so that the LOD can know about them too and is able to request content on those children tiles as well as needed/triggered by the cesium LOD request mechanism.

• Brief description about transcoding. We use web workers to transcode I3S geometries into glTF.

  • the steps are:
    • Decode geometry attributes (positions, normals, etc..) either from DRACO or Binary format.
    • If requested ( by defining a geoidTiledTerrainProvider when creating an I3SDataProvider) convert heights for all vertices & bounding boxes of input I3S layer from its gravity related height to Ellipsoidal.
    • We then transform vertex coordinates from LONG/LAT/HEIGHT to Cartesian in local space and scale appropriately if specified in the attribute metadata
    • Crop UVs if UV regions are defined in the attribute metadata
    • Create a glTF document in memory that will be ingested as part of a glb payload

@Tamrat-B thanks for the update and detailed summary. I'm planning on reviewing in the next week or so.

@lilleyse we have addressed most of the feedbacks of the review.

Here is a summary of the changes:
-Added suite of unit tests
-Split off I3Slayer into its own file to simplify unit testing
-Refactor logic for handling url and query parameters by using the Resource class
-Remove the ability to replace existing data by loading a new url. Url is passed to constructor instead
-Move camera functionality out of I3SDataProvider
-Add show property to I3SDataProvider
-Replace all instances of null with undefined
-Clean up unused code
-Add some missing documentation

The i3s unit test coverage is generally above 93%.

There is still a culling bug where some of the tiles disappear at certain angles but the traversal issues seem generally better now.

@Tamrat-B I pushed some changes:

• Fixed the remaining culling issues by computing the position accessor min/max in decodeI3s. The min/max is used to create the draw command's bounding sphere. The previous value [0, 0, 0] was causing undefined behavior in Scene.
• General cleanup to match CesiumJS code style: using defined in more places, simplifying promises, declaring all member variables in constructor, etc.
• Some updates to the API. I3SDataProvider now takes a single options object similar to Cesium3DTileset. options.url is the only required option. The provider is loaded in the constructor now, so load() has been removed.
• Exposed more I3S classes in the documentation since they are used in the sandcastles. Most of them are marked as @internalConstructor.

I still would like to change the requestContent hook approach so that I3SNode doesn't override the Cesium3DTile prototype, I just haven't landed on a good solution yet.

Otherwise I think this is ready. Since I made a lot of changes, if you could re-test other I3S datasets that are not in the sandcastle that would be helpful, to make sure I didn't introduce any regressions.

This should be good to merge now. I still want to think about the requestContent hook it doesn't need to hold up this PR. Thanks @Tamrat-B and everyone else for this contribution!

@lilleyse @sanjeetsuhag @pjcozzi

Thanks a lot folks! This should really make a lot of users happy! Thanks again for the rigorous review and making this contribution much stronger. We'll work on maintaining and improving this together. Special thanks for Anthony Mirabeau for his contribution.